fix: avoid stale process slots after compaction - #242

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/multiprocess/multiprocess_memory_limit.c (2)

69-103: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Stale cache never gets refreshed after cross-process compaction.

When another process compacts this slot away, the fallback loop deliberately leaves region_info.my_slot unrefreshed "Do not rewrite my_slot here: other threads may still be using the stale cache." That means every subsequent call from this process (a hot path used by memory accounting and status checks) pays an O(proc_num) linear scan for the remainder of the process's life, since nothing else ever repairs the cache for this process. Consider refreshing the cache with an atomic pointer store (atomic_store_explicit on region_info.my_slot as _Atomic(shrreg_proc_slot_t*)) instead of leaving it permanently stale — that resolves both the thread-safety concern noted in the comment and the perpetual fallback cost.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/multiprocess/multiprocess_memory_limit.c` around lines 69 - 103, Update
get_current_proc_slot so that, after the fallback scan finds the current process
slot, it refreshes region_info.my_slot via an atomic pointer store using the
field’s _Atomic(shrreg_proc_slot_t*) type. Preserve the existing validation and
return behavior while ensuring subsequent calls avoid repeatedly scanning
proc_num entries.

---

932-937: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated compaction block — extract a helper.

The 5-line sequence (copy_proc_slot_atomic + region_info.my_slot fixup + clear_proc_slot_atomic) is repeated verbatim for the PID=0 branch and the dead-proc branch. Extracting it reduces the risk of the two copies drifting apart on a future edit (e.g., the seqlock-protocol fix above would otherwise need to be applied twice).

♻️ Suggested helper

+static inline void compact_last_slot_into(shared_region_t* region, int target_slot) {
+    shrreg_proc_slot_t* last_slot = &region->procs[region->proc_num];
+    copy_proc_slot_atomic(&region->procs[target_slot], last_slot);
+    if (region_info.my_slot != NULL && region_info.my_slot == last_slot) {
+        region_info.my_slot = &region->procs[target_slot];
+    }
+    clear_proc_slot_atomic(last_slot);
+}

Then both branches reduce to region->proc_num--; compact_last_slot_into(region, slot); __sync_synchronize();.

Also applies to: 951-956

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/multiprocess/multiprocess_memory_limit.c` around lines 932 - 937, Extract
the duplicated compaction sequence into a helper such as compact_last_slot_into,
preserving the copy_proc_slot_atomic call, region_info.my_slot fixup, and
clear_proc_slot_atomic call. Replace the corresponding PID=0 and dead-proc
branches with region->proc_num--, the helper call, and the existing
synchronization, ensuring both paths share identical compaction behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes